Skip to content

HDDS-14198. Reduce parameter count in BucketEndpoint#9528

Merged
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-14198
Dec 19, 2025
Merged

HDDS-14198. Reduce parameter count in BucketEndpoint#9528
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-14198

Conversation

@adoroszlai
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Reduce parameter count in BucketEndpoint request handler by getting query parameters from the request context.

Benefits:

  • Prepare for refactoring into separate handler classes (HDDS-14123, etc.). Each handler will need to look up only the parameters it cares about, no need to pass everything from BucketEndpoint.
  • Eliminate the parameternumber warning.
  • Make method calls in unit tests easier to understand.

CC @echonesis

https://issues.apache.org/jira/browse/HDDS-14198

How was this patch tested?

Ran checkstyle and all S3G unit/integration tests locally.

CI:
https://github.com/adoroszlai/ozone/actions/runs/20363080334

@adoroszlai adoroszlai self-assigned this Dec 19, 2025
@adoroszlai adoroszlai added s3 S3 Gateway code-cleanup Changes that aim to make code better, without changing functionality. labels Dec 19, 2025
Copy link
Copy Markdown
Contributor

@sodonnel sodonnel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The only thing I thought was that its a little strange that all the params get removed from the methods parameter list except those with defaults. I'm fine with them staying like this, but is another option to put the default into the getQueryParam() method and overload it? Eg:

getQueryParam(QueryParams.CONTINUATION_TOKEN);

or

getQueryParam(QueryParams.QueryParams.MAX_KEYS, 1000);

This comment is optional, only do it if you think it makes sense, in this or another PR.

@adoroszlai
Copy link
Copy Markdown
Contributor Author

Thanks @sodonnel for the review.

all the params get removed from the methods parameter list except those with defaults

Those are primitive int parameters, so we'd need further conversion from the String that comes from map of query parameters. Will look into it as a follow-up.

Copy link
Copy Markdown
Contributor

@ivandika3 ivandika3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1.

@adoroszlai adoroszlai merged commit e0d272a into apache:master Dec 19, 2025
43 checks passed
@adoroszlai adoroszlai deleted the HDDS-14198 branch December 19, 2025 16:10
@adoroszlai
Copy link
Copy Markdown
Contributor Author

Thanks @ivandika3, @rich7420, @sodonnel for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-cleanup Changes that aim to make code better, without changing functionality. s3 S3 Gateway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants